AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

Skipped (out of scope): dist/, pnpm-lock.yaml

Findings

🔶 Medium — CHANGELOG entry placed in released version section

File: packages/pluggableWidgets/datagrid-web/CHANGELOG.md line 51
Problem: The new entry "We improved screen reader support for selection feature." was added under ## [3.8.0] - 2026-01-16, which is an already-released version. Consumers reading the changelog will see it as a 3.8.0 change, not an upcoming one.
Fix: Move the entry into the ## [Unreleased] section at the top, under a ### Changed heading:

## [Unreleased]

### Changed

- We improved screen reader support for selection feature.

🔶 Medium — SelectionAriaLive announces the wrong text when "select all pages" is active

File: packages/pluggableWidgets/datagrid-web/src/features/selection-counter/SelectionAriaLive.tsx line 9
Problem: The live region reads texts.selectedCountText (e.g. "100 items selected") from selectionCounterTextsStore. But selectAllTextsStore.selectionStatus already handles the "all pages selected" case separately — it returns allSelectedText (e.g. "All 100 rows selected.") when isAllItemsSelected is true, and delegates to selectedCountText otherwise. By bypassing selectionStatus, the live region announces a generic count even when the user has selected all rows across pages, so the screen reader announcement doesn't match the visual message in SelectAllBarViewModel.selectionStatus.
Fix: Inject CORE_TOKENS.selection.selectAllTexts and use its selectionStatus property:

import { observer } from "mobx-react-lite";
import { useSelectAllTexts } from "../../model/hooks/injection-hooks";

export const SelectionAriaLive = observer(function SelectionAriaLive() {
    const texts = useSelectAllTexts();

    return (
        <span className="sr-only" aria-live="polite" aria-atomic="true">
            {texts.selectionStatus}
        </span>
    );
});

🔶 Medium — No unit tests for SelectionAriaLive

File: packages/pluggableWidgets/datagrid-web/src/features/selection-counter/SelectionAriaLive.tsx
Problem: The new component is the primary accessibility deliverable of this PR and has no test coverage. At minimum, tests should verify: (1) aria-live="polite" and aria-atomic="true" are present, (2) the announced text matches the injected store value, (3) it renders an empty string when selectedCountText is "" (zero selection).
Fix: Add a spec at src/features/selection-counter/__tests__/SelectionAriaLive.spec.tsx using RTL and brandi injection helpers, following the pattern of existing tests in this package.

⚠️ Low — useSelectAllTexts is exported but never consumed

File: packages/pluggableWidgets/datagrid-web/src/model/hooks/injection-hooks.ts line 30
Note: The hook is added in this PR but has no import site anywhere in the package. If the fix for the SelectionAriaLive issue above is applied (using selectAllTexts.selectionStatus), this hook becomes its consumer. Otherwise, remove it to avoid dead exports.

Positives

• Separating the live region into a dedicated sr-only component is the right architectural move — it eliminates the previous pattern of putting aria-live on a visible span, which can cause double-reading by some screen readers.
• observer HOC is correctly applied to SelectionAriaLive, ensuring MobX reactivity.
• Both aria-live="polite" and aria-atomic="true" are set, which is the correct combination for a selection counter (avoids interrupting ongoing announcements, and announces the full text atomically).
• Moving selectAllTextsStore into the core selection.model and consolidating the token under CORE_TOKENS is a clean cohesion improvement — the store was conceptually always core selection logic, not select-all-module logic.
• The injection chain for selectAllTextsStore in Root.container.ts is correctly ordered with all dependencies declared via injected(...) before the container binding.